Notification docs#886
Conversation
iaroslav-ciupin
requested review from
EngHabu,
cosmicBboy,
kumare3,
ppiegaze and
samhita-alla
as code owners
Deploying docs with
|
| Latest commit: |
5d496d9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f9150d8f.docs-dog.pages.dev |
| Branch Preview URL: | https://notification-docs.docs-dog.pages.dev |
There was a problem hiding this comment.
Pull request overview
Adds end-user documentation for configuring Flyte notifications both on a per-run basis and on scheduled triggers.
Changes:
- Introduces a new “Run with notifications” guide showing
flyte.with_runcontext(notifications=...)usage. - Adds a new “Notifications” section to trigger configuration docs, including examples for Slack/Email/Teams/Webhook.
- Documents execution phases and message templating variables for notifications.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
content/user-guide/task-deployment/run-with-notifications.md |
New page explaining how to attach notifications to a single run and providing Slack/Email examples. |
content/user-guide/task-configuration/triggers.md |
Adds a “Notifications” section describing trigger-level notifications and examples for different delivery channels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Template variables | ||
|
|
||
| All message fields support template variables that are substituted at delivery time: | ||
|
|
||
| | Variable | Description | | ||
| |--------------------|--------------------------------------------------------| | ||
| | `{{.Run.Project}}` | Project name | | ||
| | `{{.Run.Domain}}` | Domain name | | ||
| | `{{.Run.Name}}` | Run ID | | ||
| | `{{.Phase}}` | Execution phase | | ||
| | `{{.Error}}` | Error message when failed or abort reason whan aborted | |
There was a problem hiding this comment.
The template variable syntax and names here (e.g., {{.Run.Name}}, {{.Error}}) don’t match the Flyte Notifications API reference in this repo, which documents Python-format placeholders like {run.name}, {run.error}, {run.url}, {project}, {domain} (see content/api-reference/flyte-sdk/packages/flyte.notify/_index.md). As written, users will likely get literal {{...}} text in delivered notifications. Please align the variable table (and the examples in this section) with the documented placeholder format/keys.
| message="Run {{.Run.Name}} failed with: {{.Error}}", | ||
| ), | ||
| notify.Email( | ||
| on_phase=ActionPhase.SUCCEEDED, | ||
| recipients=["[email protected]"], | ||
| subject="Run {{.Run.Name}} succeeded", | ||
| body="Run: {{.Run.Name}}", |
There was a problem hiding this comment.
This example message uses {{.Run.Name}} / {{.Error}} placeholders, but the Flyte Notifications API reference in this repo documents {run.name} / {run.error} (and similar) placeholders. Please update the example to match the actual template syntax so it works when copy/pasted.
| message="Run {{.Run.Name}} failed with: {{.Error}}", | |
| ), | |
| notify.Email( | |
| on_phase=ActionPhase.SUCCEEDED, | |
| recipients=["[email protected]"], | |
| subject="Run {{.Run.Name}} succeeded", | |
| body="Run: {{.Run.Name}}", | |
| message="Run {run.name} failed with: {run.error}", | |
| ), | |
| notify.Email( | |
| on_phase=ActionPhase.SUCCEEDED, | |
| recipients=["[email protected]"], | |
| subject="Run {run.name} succeeded", | |
| body="Run: {run.name}", |
| | `{{.Run.Domain}}` | Domain name | | ||
| | `{{.Run.Name}}` | Run ID | | ||
| | `{{.Phase}}` | Execution phase | | ||
| | `{{.Error}}` | Error message when failed or abort reason whan aborted | |
There was a problem hiding this comment.
Typo in the template variable description: “whan” → “when”.
| | `{{.Error}}` | Error message when failed or abort reason whan aborted | | |
| | `{{.Error}}` | Error message when failed or abort reason when aborted | |
|
|
||
| ```python | ||
| import flyte | ||
| from flyte import notify |
There was a problem hiding this comment.
The snippets import notifications as from flyte import notify, but elsewhere in this docs repo the documented pattern is import flyte.notify as notify (e.g., content/api-reference/flyte-sdk/packages/flyte/_index.md:820-822). Using the module import form avoids relying on flyte re-exporting notify and is more consistent for readers.
| from flyte import notify | |
| import flyte.notify as notify |
| ## Template variables | ||
|
|
||
| All message fields support template variables substituted at delivery time: | ||
|
|
||
| | Variable | Description | | ||
| |----------|-------------| | ||
| | `{{.Run.Project}}` | Project name | | ||
| | `{{.Run.Domain}}` | Domain name | | ||
| | `{{.Run.Name}}` | Run ID | | ||
| | `{{.Phase}}` | Execution phase | | ||
| | `{{.Error}}` | Error message (failed) or abort reason (aborted) | |
There was a problem hiding this comment.
The template variable syntax and keys in this page (e.g., {{.Run.Name}}, {{.Phase}}, {{.Error}}) don’t match the Flyte Notifications API reference in this repo, which documents placeholders like {run.name}, {run.phase}, {run.error}, {run.url}, {project}, {domain} (see content/api-reference/flyte-sdk/packages/flyte.notify/_index.md). As written, notification messages will likely not render as intended when users copy/paste them. Please update the examples and the variable table to the documented placeholder format.
| message="Run {{.Run.Name}} succeeded.", | ||
| ), | ||
| notify.Email( | ||
| on_phase=ActionPhase.FAILED, | ||
| recipients=[NOTIFICATION_EMAIL], | ||
| subject="ALERT: Run {{.Run.Name}} failed", | ||
| body="Run: {{.Run.Name}}\nError: {{.Error}}", |
There was a problem hiding this comment.
This example uses {{.Run.Name}} / {{.Error}} placeholders, but the Flyte Notifications API reference in this repo shows {run.name} / {run.error} placeholders for notification message templating. Please update the example strings so they match the real template syntax.
| message="Run {{.Run.Name}} succeeded.", | |
| ), | |
| notify.Email( | |
| on_phase=ActionPhase.FAILED, | |
| recipients=[NOTIFICATION_EMAIL], | |
| subject="ALERT: Run {{.Run.Name}} failed", | |
| body="Run: {{.Run.Name}}\nError: {{.Error}}", | |
| message="Run {run.name} succeeded.", | |
| ), | |
| notify.Email( | |
| on_phase=ActionPhase.FAILED, | |
| recipients=[NOTIFICATION_EMAIL], | |
| subject="ALERT: Run {run.name} failed", | |
| body="Run: {run.name}\nError: {run.error}", |
| print(f"Result: {result}") | ||
| ``` | ||
|
|
||
| Pass a single notification or a tuple of notifications. All notification types from `flyte.notify` are supported: `Slack`, `Email`, `Teams`, `Webhook`, and `NamedDelivery`. |
There was a problem hiding this comment.
This line says “All notification types from flyte.notify are supported” but the flyte.notify module also documents types like NamedRule in the API reference. Either include all supported types (including whether NamedRule is supported here) or reword this to avoid an exhaustive claim (e.g., “Common notification types include ...”).
| Pass a single notification or a tuple of notifications. All notification types from `flyte.notify` are supported: `Slack`, `Email`, `Teams`, `Webhook`, and `NamedDelivery`. | |
| Pass a single notification or a tuple of notifications. Common notification types from `flyte.notify` include: `Slack`, `Email`, `Teams`, `Webhook`, and `NamedDelivery`. |
| # Python >= 3.12 | ||
| pip install aiosmtpd | ||
| sudo python -m aiosmtpd -n -l localhost:25 | ||
| ``` |
There was a problem hiding this comment.
For local SMTP testing, recommending sudo python ... localhost:25 encourages running Python as root. Since the text already mentions 1025 as an alternative, consider making the non-root option the default command (and mention 25 only as an optional privileged-port variant).
| ```python | ||
| import os | ||
| import flyte | ||
| from flyte import notify |
There was a problem hiding this comment.
The snippets import notifications as from flyte import notify, but the API reference examples use import flyte.notify as notify (e.g., content/api-reference/flyte-sdk/packages/flyte/_index.md:820-822). Consider switching to the module import form to avoid relying on a re-export and to match the documented style.
| from flyte import notify | |
| import flyte.notify as notify |
iaroslav-ciupin
added
the
do-not-merge
DO NOT MERGE THIS PR YET!